-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve screen repaint speed #45
base: master
Are you sure you want to change the base?
Conversation
@malcomvetter thanks for the PR. Can you please look into the test failures? I'll also appreciate some data on how much your changes improve repaint speed |
Hey @tonerdo, I haven't tried out @malcomvetter's changes, but I have been noticing some screen re-paint issues for tab completion in a project I'm working on that uses your readline library. I think the issue stems from the fact that when tabbing through options, they are erased one character at a time. It's not always an issue, but I've noticed it particularly when I'm using my application over an SSH connection. I think erasing the string all at once while tabbing through options would make a big difference. Anyway, thanks for the awesome project, I've been getting a ton of usage out of it! |
That’s essentially what my changes do. They break some unit tests (due to
refactor) but I haven’t gotten around to fixing that.
…On Thu, Sep 6, 2018 at 1:54 PM Ryan Cobb ***@***.***> wrote:
Hey @tonerdo <https://github.com/tonerdo>, I haven't tried out
@malcomvetter <https://github.com/malcomvetter>'s changes, but I have
been noticing some screen re-paint issues for tab completion in a project
I'm working on that uses your readline library.
I think the issue stems from the fact that when tabbing through options,
they are erased one character at a time. It's not always an issue, but I've
noticed it particularly when I'm using my application over an SSH
connection.
I think erasing the string all at once while tabbing through options would
make a big difference. Anyway, thanks for the awesome project, I've been
getting a ton of usage out of it!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#45 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AMJRZUn-vJ_5OWMIN1fcCQn--4pPfwAjks5uYW-BgaJpZM4U_UvB>
.
|
@malcomvetter Yeah, I am hoping it will get merged for easy nuget restore :) |
Open a PR and make sure it doesn't fail like I am seeing currently. Test it out.. if it works better than the previous ReadLine build, it should be good to go. |
This PR breaks multiline editing in many ways. There was a reason the original code "moved char by char". To migrate it to move "faster" the "rendering" code must be a lot more clever. For example: when moving left this PR tests if the cursor is at the "prompt" length and prevent it from going further. This only works on the first line. |
Ideally, I'd like to see conversion to Generic Lists over arrays everywhere, but these mods made this library much easier to integrate into one of my other projects.
The other issue is that by keeping track of the cursor pointer and changing one char at a time, it's slow and the repaint factor is frustrating for a user used to the native GNU tools on *nix. I modified the logic to cut to the chase and quickly repaint the affected text whole strings at a time instead of per char.
Neat project overall-- thanks for sharing. I hope these changes are helpful to your goals.